-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[NFC][SpecialCaseList] Hide Section internals in private section #167276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NFC][SpecialCaseList] Hide Section internals in private section #167276
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-clang Author: Vitaly Buka (vitalybuka) ChangesPreparing to moving most of implementation out of the header file. Full diff: https://github.com/llvm/llvm-project/pull/167276.diff 5 Files Affected:
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 2dec26ecacf26..5e9da245e2b43 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -534,7 +534,7 @@ WarningsSpecialCaseList::create(const llvm::MemoryBuffer &Input,
void WarningsSpecialCaseList::processSections(DiagnosticsEngine &Diags) {
static constexpr auto WarningFlavor = clang::diag::Flavor::WarningOrError;
for (const auto &SectionEntry : sections()) {
- StringRef DiagGroup = SectionEntry.SectionStr;
+ StringRef DiagGroup = SectionEntry.name();
if (DiagGroup == "*") {
// Drop the default section introduced by special case list, we only
// support exact diagnostic group names.
diff --git a/clang/lib/Basic/ProfileList.cpp b/clang/lib/Basic/ProfileList.cpp
index 9cb118893a0d9..8727057eb78d1 100644
--- a/clang/lib/Basic/ProfileList.cpp
+++ b/clang/lib/Basic/ProfileList.cpp
@@ -36,7 +36,7 @@ class ProfileSpecialCaseList : public llvm::SpecialCaseList {
bool hasPrefix(StringRef Prefix) const {
for (const auto &It : sections())
- if (It.Entries.count(Prefix) > 0)
+ if (It.hasPrefix(Prefix))
return true;
return false;
}
diff --git a/clang/lib/Basic/SanitizerSpecialCaseList.cpp b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
index 56f551628cf89..928c086898097 100644
--- a/clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -42,7 +42,7 @@ void SanitizerSpecialCaseList::createSanitizerSections() {
SanitizerMask Mask;
#define SANITIZER(NAME, ID) \
- if (S.SectionMatcher.matchAny(NAME)) \
+ if (S.matchName(NAME)) \
Mask |= SanitizerKind::ID;
#define SANITIZER_GROUP(NAME, ID, ALIAS) SANITIZER(NAME, ID)
@@ -68,7 +68,7 @@ SanitizerSpecialCaseList::inSectionBlame(SanitizerMask Mask, StringRef Prefix,
if (S.Mask & Mask) {
unsigned LineNum = S.S.getLastMatch(Prefix, Query, Category);
if (LineNum > 0)
- return {S.S.FileIdx, LineNum};
+ return {S.S.fileIndex(), LineNum};
}
}
return NotFound;
diff --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h
index 5ed7adeaf6c92..120d67377c674 100644
--- a/llvm/include/llvm/Support/SpecialCaseList.h
+++ b/llvm/include/llvm/Support/SpecialCaseList.h
@@ -195,17 +195,22 @@ class SpecialCaseList {
using SectionEntries = StringMap<StringMap<Matcher>>;
protected:
- struct Section {
+ class Section {
+ public:
Section(StringRef Str, unsigned FileIdx, bool UseGlobs)
: SectionMatcher(UseGlobs, /*RemoveDotSlash=*/false), SectionStr(Str),
FileIdx(FileIdx) {}
Section(Section &&) = default;
- Matcher SectionMatcher;
- SectionEntries Entries;
- std::string SectionStr;
- unsigned FileIdx;
+ // Return name of the section, it's entire string in [].
+ StringRef name() const { return SectionStr; }
+
+ // Returns true of string 'Name' matches section name interpreted as a glob.
+ LLVM_ABI bool matchName(StringRef Name) const;
+
+ // Return sequence number of the file where this section is defined.
+ unsigned fileIndex() const { return FileIdx; }
// Helper method to search by Prefix, Query, and Category. Returns
// 1-based line number on which rule is defined, or 0 if there is no match.
@@ -217,11 +222,19 @@ class SpecialCaseList {
LLVM_ABI StringRef getLongestMatch(StringRef Prefix, StringRef Query,
StringRef Category) const;
+ /// Returns true if the section has any entries for the given prefix.
+ LLVM_ABI bool hasPrefix(StringRef Prefix) const;
+
private:
friend class SpecialCaseList;
LLVM_ABI void preprocess(bool OrderBySize);
LLVM_ABI const SpecialCaseList::Matcher *
findMatcher(StringRef Prefix, StringRef Category) const;
+
+ Matcher SectionMatcher;
+ std::string SectionStr;
+ SectionEntries Entries;
+ unsigned FileIdx;
};
ArrayRef<const Section> sections() const { return Sections; }
diff --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp
index 8e6e9f34a73f3..395a55d75acd4 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -358,6 +358,10 @@ SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
return NotFound;
}
+bool SpecialCaseList::Section::matchName(StringRef Name) const {
+ return SectionMatcher.matchAny(Name);
+}
+
const SpecialCaseList::Matcher *
SpecialCaseList::Section::findMatcher(StringRef Prefix,
StringRef Category) const {
@@ -394,4 +398,8 @@ StringRef SpecialCaseList::Section::getLongestMatch(StringRef Prefix,
return {};
}
+bool SpecialCaseList::Section::hasPrefix(StringRef Prefix) const {
+ return Entries.find(Prefix) != Entries.end();
+}
+
} // namespace llvm
|
Preparing to moving most of implementation out of the header file. Pull Request: llvm#167276
Created using spr 1.3.7 [skip ci]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the SpecialCaseList::Section from a struct with public members to a class with private data members and public accessor methods. This is preparation work for moving most of the implementation out of the header file.
- Changed
Sectionfrom struct to class with encapsulated data members - Added accessor methods:
name(),matchName(),fileIndex(), andhasPrefix() - Updated all client code to use the new accessor methods instead of direct member access
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| llvm/include/llvm/Support/SpecialCaseList.h | Converted Section from struct to class, moved data members to private, and added public accessor methods |
| llvm/lib/Support/SpecialCaseList.cpp | Implemented the new accessor methods matchName() and hasPrefix() |
| clang/lib/Basic/SanitizerSpecialCaseList.cpp | Updated to use matchName() and fileIndex() accessor methods |
| clang/lib/Basic/ProfileList.cpp | Updated to use hasPrefix() accessor method |
| clang/lib/Basic/Diagnostic.cpp | Updated to use name() accessor method |
Comments suppressed due to low confidence (1)
llvm/lib/Support/SpecialCaseList.cpp:345
- Inconsistent encapsulation: Direct access to
S.SectionMatcheron line 342 andS.FileIdxon line 345 bypasses the new accessor methods. Should useS.matchName(Section)andS.fileIndex()respectively to maintain consistent encapsulation introduced by this refactoring.
if (S.SectionMatcher.matchAny(Section)) {
unsigned Blame = S.getLastMatch(Prefix, Query, Category);
if (Blame)
return {S.FileIdx, Blame};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Preparing to moving most of implementation out of the header file.